-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor #32
base: main
Are you sure you want to change the base?
Conversation
ccoVeille
commented
Sep 18, 2024
- chore: refactor code
- fix: missing check on lower boundary for Int64
- fix: underflow with min int64 with floats
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #32 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 3 3
Lines 114 129 +15
=========================================
+ Hits 114 129 +15 ☔ View full report in Codecov by Sentry. |
- remove assertNotNegative - use checkLowerBoundary
it doesn't underflow due to float imprecision
// the number was between 0.99 and 1.01 of the minimum value | ||
// once converted back and forth, we need to check if the value is in the same range | ||
// if not, so it's an underflow | ||
return conv >= b*0.99 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that can't be correct even with a lot more nines?
I suggest you write a function to find how many mantisse bits there are in float64 (the answer is 53 but it's better to "prove" it) and float32 and use that as boundary tests (tests, not for runtime, imo, the runtime is covered by the roundtrip conversion, afaik)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrote such function in fortio/safecast#8
or you can hardcode 53 and 24 bits as defined by the IEEE standard.